Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --from-post=<post_id> flag to create duplicate posts #154

Merged
merged 9 commits into from May 8, 2018

Conversation

sagarnasit
Copy link
Contributor

I here modified the create command to generate a duplicate post.

Copy link
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Great start! Can you include functional tests too?

*
* [--from-post=<post_id>]
* : The post id of a post to be duplicated.
*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this is spaces instead of tabs. Can you correct to tabs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I will correct it.

* # Create a duplicate post with same post data.
* $ wp post create --from-post=123 --post_title='Different Title'
* Success: Created post 2350.
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto spaces -> tabs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad. I will change it too.

$post_id = $post_arr['ID'];
unset( $post_arr['post_date'] );
unset( $post_arr['post_date_gmt'] );
unset( $post_arr['guid'] );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WHy did you decide to skip these values?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This values will be duplicated too. I skip post_date and post_date_gmt so that values can be reset according to the current date at the time of duplicating. Reason to unset guid is that duplicated post guid will same as the original.

If you have any suggestions please let me know.

if( isset( $assoc_args['from-post'] ) ) {
$post = $this->fetcher->get_check( $assoc_args['from-post'] );
$post_arr = get_object_vars( $post );
$post_id = $post_arr['ID'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$post_arr['ID'] should be unset, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, you are right. Actually, if we don't unset id it will still generate new id. But, for a safer side, I will make correct it.

* @param $post_id id of the post.
* @return array
*/
private function get_metadata( $post_id = null ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of making $post_id optional, we should make it required:

get_metadata( $post_id = null )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure.


if ( empty( $assoc_args['meta_input'] ) ) {
$assoc_args['meta_input'] = $this->get_metadata( $post_id );
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to persist taxonomy data too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is for overwriting existing metadata if a user wants. Categories and tags can be duplicated too. what are your thoughts about it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think categories and tags should be duplicated too.

@sagarnasit
Copy link
Contributor Author

@danielbachhuber I made changes you asked and added taxonomy duplication as well. I don't have any idea about functional testing so I haven't done testing yet.

@schlessera
Copy link
Member

schlessera commented Mar 22, 2018

@sagarnasit I can help you go through setting up the testing.

You should first check whether you can even run the existing tests.

As the Functional Tests help states, you'll need to prepare your database server first:

Before running the functional tests, you’ll need a MySQL (or MariaDB) user called wp_cli_test with the password password1 that has full privileges on the MySQL database wp_cli_test. Running the following as root in MySQL should do the trick:

GRANT ALL PRIVILEGES ON wp_cli_test.* TO "wp_cli_test"@"localhost" IDENTIFIED BY "password1";

Then, to run the entire test suite:

./vendor/bin/behat --expand

Or to test a single feature:

./vendor/bin/behat features/core.feature

@schlessera
Copy link
Member

schlessera commented Apr 2, 2018

Looking into the Travis failure: #161

@schlessera
Copy link
Member

@sagarnasit I should have addressed the test failure on PHP 5.3 now. Can you merge latest master into your PR to make the tests pass again?

@sagarnasit
Copy link
Contributor Author

sagarnasit commented Apr 14, 2018

Thanks @schlessera . I also added functional testing as @danielbachhuber said.

"""
{TERM_ID}
"""
@require-wp-4.4
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blank line needed between scenarios (above the @require-wp-4.4 tag).

@@ -171,7 +178,28 @@ public function create( $args, $assoc_args ) {
}

$array_arguments = array( 'meta_input' );
$assoc_args = \WP_CLI\Utils\parse_shell_arrays( $assoc_args, $array_arguments );
$assoc_args = \WP_CLI\Utils\parse_shell_arrays($assoc_args, $array_arguments);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This entire block (lines 181-202) seems to have been inadvertently reformatted to PSR2 with the last commit.

Please adapt the formatting to adhere to WPCS instead.

*
* @return array
*/
private function get_tags( $post_id ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code path for tags is not covered by the tests.

@schlessera
Copy link
Member

@sagarnasit This looks good already. There's some whitespace that got messed up during the last commit that you need to redress, and I would like to also have a test that covers the "tags" code.

@sagarnasit
Copy link
Contributor Author

@schlessera Added Test for Tags. Looks Good?

Copy link
Member

@schlessera schlessera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good. Only some minor stuff as a final cleanup, and then we can merge.

/**
* Get Tags of a post.
*
* @param $post_id postid of the post.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be:

@param $post_id ID of the post.

/**
* Get Categories of a post.
*
* @param $post_id postid of the post.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be:

@param $post_id ID of the post.

/**
* Get post metadata.
*
* @param $post_id id of the post.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be:

@param $post_id ID of the post.


When I run `wp post term add {POST_ID} post_tag {TAG_ID} --by=id`
Then STDOUT should contain:
"""*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a random asterisk added here.

@sagarnasit
Copy link
Contributor Author

@schlessera Made changes you asked for.

@schlessera schlessera changed the title create subcommand extended to generate duplicate post Add --from-post<post_id> flag to create duplicate posts May 8, 2018
@schlessera schlessera changed the title Add --from-post<post_id> flag to create duplicate posts Add --from-post=<post_id> flag to create duplicate posts May 8, 2018
@schlessera schlessera merged commit cf1741d into wp-cli:master May 8, 2018
@schlessera schlessera added this to the 1.3.0 milestone May 8, 2018
@schlessera
Copy link
Member

Thank you for the pull request, @sagarnasit !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants